Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.3] Update Connection.php #16422

Merged
merged 1 commit into from
Nov 15, 2016
Merged

[5.3] Update Connection.php #16422

merged 1 commit into from
Nov 15, 2016

Conversation

dlashua
Copy link
Contributor

@dlashua dlashua commented Nov 15, 2016

reconnectIfMissingConnection() calls getPdo(). This executes the callback which is not desired for read/write connections where the read is being used. Checking is_null($this->pdo) instead allows for the callback to not be called until it is needed.

reconnectIfMissingConnection() calls getPdo(). This executes the callback which is not desired for read/write connections where the read is being used. Checking is_null($this->pdo) instead allows for the callback to not be called until it is needed.
@taylorotwell
Copy link
Member

This issue needs more explanation. I don't think your change looks correct. $this->pdo could be a Closure.

@dlashua
Copy link
Contributor Author

dlashua commented Nov 15, 2016

That's exactly the point. When $this->getPdo() is called, the closure is called. However, for connections with separate read/write servers that closure shouldn't be called unless a write connection is needed. If $this->getPdo() is called, then the closure is called. So reconnectIfMissingConnection() needs to avoid calling $this->getPdo(). By looking at $this->pdo instead of $this->getPdo() the same result is obtained (see the code for getPdo()) without actually calling the closure.

Without this patch, the write connection is connected to every time even when a query that should hit the read connection is being run.

@BrandonSurowiec
Copy link
Contributor

Issue: #15359

@taylorotwell taylorotwell merged commit cae2043 into laravel:5.3 Nov 15, 2016
@GrahamCampbell GrahamCampbell changed the title Update Connection.php [5.3] Update Connection.php Nov 15, 2016
GrahamCampbell referenced this pull request Dec 28, 2016
issue #15359
fixed in version 5.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants